Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Catch an error that is reported, but silently ignored during CI testing #2665

Merged
merged 3 commits into from
Jun 19, 2024
Merged

Catch an error that is reported, but silently ignored during CI testing #2665

merged 3 commits into from
Jun 19, 2024

Conversation

freakboy3742
Copy link
Member

@freakboy3742 freakboy3742 commented Jun 17, 2024 •
edited
Loading

Close inspection of a CI run for the Winforms testbed shows the following:

tests\widgets\test_table.py::test_headerless_column_changes PASSED       [ 75%]
tests\widgets\test_table.py::test_remove_all_columns Traceback (most recent call last):
  File "D:\a\toga\toga\testbed\build\testbed\windows\app\src\app_packages\toga_winforms\widgets\table.py", line 176, in _new_item
    icon = icon(self._accessors[0])
  File "D:\a\toga\toga\testbed\build\testbed\windows\app\src\app_packages\toga_winforms\widgets\table.py", line 90, in winforms_retrieve_virtual_item
    e.Item = self._new_item(e.ItemIndex)
  File "D:\a\toga\toga\testbed\build\testbed\windows\app\src\app_packages\toga_winforms\libs\wrapper.py", line 22, in __call__
    return function(*args, **kwargs)
   at Python.Runtime.PythonException.ThrowLastAsClrException() in /tmp/build-via-sdist-h3j67_wc/pythonnet-3.0.3/src/runtime/PythonException.cs:line 53

   at Python.Runtime.Dispatcher.TrueDispatch(Object[] args) in /tmp/build-via-sdist-h3j67_wc/pythonnet-3.0.3/src/runtime/DelegateManager.cs:line 341

   at Python.Runtime.Dispatcher.Dispatch(Object[] args) in /tmp/build-via-sdist-h3j67_wc/pythonnet-3.0.3/src/runtime/DelegateManager.cs:line 208

   at __System_Windows_Forms_RetrieveVirtualItemEventHandlerDispatcher.Invoke(Object , RetrieveVirtualItemEventArgs )

   at System.Windows.Forms.ListView.OnRetrieveVirtualItem(RetrieveVirtualItemEventArgs e)

   at System.Windows.Forms.ListView.WmReflectNotify(Message& m)

   at System.Windows.Forms.ListView.WndProc(Message& m)

   at System.Windows.Forms.NativeWindow.Callback(IntPtr hWnd, Int32 msg, IntPtr wparam, IntPtr lparam)
list index out of range
PASSED              [ 75%]
tests\widgets\test_table.py::test_cell_icon PASSED                       [ 76%]

tests\widgets\test_table.py::test_remove_all_columns is passing but raising an exception that is being ignored. The error is caused when the all columns are removed from the table, it no longer has accessors, so looking up column 0 to render the table's icon fails. However, because this exception occurs in a callback, it doesn't surface as a full error.

This PR catches and ignores the error.

PR Checklist:

  • All new features have been tested
  • All new features have been documented
  • I have read the CONTRIBUTING.md file
  • I will abide by the code of conduct

mhsmith
mhsmith reviewed Jun 18, 2024
View reviewed changes
winforms/src/toga_winforms/widgets/table.py Outdated
Comment on lines 176 to 178
try:
icon = icon(self._accessors[0])
except IndexError:
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This could hide other IndexErrors from inside icon(), so in this case I think it's better to ask for permission rather than forgiveness.

@mhsmith mhsmith merged commit 9a7b2bf into beeware:main Jun 19, 2024
31 of 34 checks passed
@freakboy3742 freakboy3742 deleted the winforms-test-error branch June 19, 2024 21:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@mhsmith mhsmith mhsmith approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@freakboy3742 @mhsmith